Skip to content

x/exp/schema: fix reserved keyword handling and add validation#130

Merged
patjakdev merged 1 commit intocedar-policy:mainfrom
strongdm:patjak/schema-fixes
Feb 10, 2026
Merged

x/exp/schema: fix reserved keyword handling and add validation#130
patjakdev merged 1 commit intocedar-policy:mainfrom
strongdm:patjak/schema-fixes

Conversation

@patjakdev
Copy link
Copy Markdown
Collaborator

@patjakdev patjakdev commented Feb 10, 2026

Issue #, if available:

Description of changes:
Add a tokenReservedKeyword token type to the schema parser's lexer, matching the approach used in the main Cedar parser. Previously, reserved keywords like "true", "false", "in", "if", etc. were lexed as plain identifiers, which meant the parser silently accepted them in positions where they should be rejected (e.g. entity true;, type if = String;).

Bugs fixed:

  • Reserved Cedar keywords were accepted as entity, type, and action names, namespace path components, and attribute names without quoting. The parser now rejects these with a clear error message.
  • __cedar as a definition name (entity, type, enum) was silently accepted. These are now rejected while still allowing __cedar as an action name, attribute name, and type reference prefix, which matches the Cedar Rust behavior.
  • Duplicate annotations (e.g. @doc("a") @doc("b")) were silently accepted with last-wins semantics. The parser now rejects duplicates.
  • Duplicate principal, resource, or context declarations within appliesTo were silently accepted. The parser now rejects duplicates.
  • Empty principal or resource type lists in appliesTo (e.g. principal: []) were silently accepted, producing a meaningless empty list. The parser now rejects these.
  • appliesTo blocks missing a principal or resource declaration were accepted. The parser now requires both.
  • MarshalSchema() emitted reserved keywords as bare identifiers in attribute and action names (e.g. true: String), producing output that could not be re-parsed. isValidIdent() now checks for reserved keywords and the marshaler quotes them.

Add a tokenReservedKeyword token type to the schema parser's lexer, matching the approach used in the main Cedar parser. Previously, reserved keywords like "true", "false", "in", "if", etc. were lexed as plain identifiers, which meant the parser silently accepted them in positions where they should be rejected (e.g. `entity true;`, `type if = String;`).

Bugs fixed:
- Reserved Cedar keywords were accepted as entity, type, and action names, namespace path components, and attribute names without quoting. The parser now rejects these with a clear error message.
- __cedar as a definition name (entity, type, enum) was silently accepted. These are now rejected while still allowing __cedar as an action name, attribute name, and type reference prefix, which matches the Cedar Rust behavior.
- Duplicate annotations (e.g. `@doc("a") @doc("b")`) were silently accepted with last-wins semantics. The parser now rejects duplicates.
- Duplicate principal, resource, or context declarations within appliesTo were silently accepted. The parser now rejects duplicates.
- Empty principal or resource type lists in appliesTo (e.g.  `principal: []`) were silently accepted, producing a meaningless empty list. The parser now rejects these.
- appliesTo blocks missing a principal or resource declaration were accepted. The parser now requires both.
- MarshalSchema emitted reserved keywords as bare identifiers in attribute and action names (e.g. `true: String`), producing output that could not be re-parsed. isValidIdent now checks for reserved keywords and the marshaler quotes them.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Signed-Off-By: Patrick Jakubowski <patrick.jakubowski@strongdm.com>
Text string
}

// N.B. "is" is included here for compatibility with the Rust implementation. The Cedar specification does not list
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is is now in the official documentation

// N.B. "is" is included here for compatibility with the Rust implementation. The Cedar specification does not list
// "is" as a reserved keyword
var reservedKeywords = []string{"true", "false", "if", "then", "else", "in", "like", "has", "is"}
var reservedKeywords = []string{"true", "false", "if", "then", "else", "in", "like", "has", "is", "__cedar"}
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At some point, they added __cedar as a reserved keyword in the policy schema

@patjakdev patjakdev marked this pull request as ready for review February 10, 2026 22:33
Copy link
Copy Markdown
Contributor

@philhassey philhassey left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@patjakdev patjakdev merged commit f4c8701 into cedar-policy:main Feb 10, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants